Skip to content

Fix/v3 tests#5996

Open
lucasjia-aws wants to merge 14 commits into
aws:masterfrom
lucasjia-aws:fix/v3-tests
Open

Fix/v3 tests#5996
lucasjia-aws wants to merge 14 commits into
aws:masterfrom
lucasjia-aws:fix/v3-tests

Conversation

@lucasjia-aws

Copy link
Copy Markdown
Collaborator

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

The dummy_container_with_user_and_workdir fixture failed to build because
the standalone `RUN apt install -y sudo` step ran several layers after the
initial apt-get update, using a stale package index and failing to connect
to deb.debian.org when fetching the sudo .deb.

Install sudo in the same layer as the initial apt-get update against a fresh
index, switch from `apt` to `apt-get`, and add Acquire::Retries so transient
network failures are retried automatically.
The judge model anthropic.claude-3-5-haiku-20241022-v1:0 has reached
end-of-life in Bedrock, causing integ test evaluation pipelines to fail
with a ValidationException. Switch to the active successor
anthropic.claude-haiku-4-5-20251001-v1:0 and add it to the allowed
evaluator models list.
… tests

Add tests/integ/conftest.py with an autouse session fixture that gives
boto3's default session adaptive retries (absorbed by the SDK's internal
IAM role-validation client), plus a makereport hook that xfails residual
SimulatePrincipalPolicy throttling under concurrent test load.
- Route Nova evaluator/recipe tests to us-east-1 via us_east_1 marker
  and us-east-1 session (InspectAI, sft max_steps below minimum)
- Pin LLMAsJudge full-flow evaluator to us-west-2 region
- Provide explicit session to no-recipe SFTTrainer test to avoid
  region resolution failure under xdist
- Correct extract_evaluator_arn integ test: Lambda ARN is rejected by
  _extract_evaluator_arn (dispatched upstream), expect ValueError
test_inspect_ai_evaluator's two tests constructed InspectAIEvaluator with
the public-hub model 'nova-textgeneration-lite' while the session-scoped
use_private_hub fixture pinned SAGEMAKER_HUB_NAME to the private 'sdktest'
hub, causing DescribeHubContent ResourceNotFound and a pydantic
ValidationError at construction time. Override SAGEMAKER_HUB_NAME to
SageMakerPublicHub in both tests, matching test_llmaj_custom_model.
The HPO, Clarify, and pipeline train-registry integ tests used hardcoded
S3 prefixes and deleted the entire prefix in their finally blocks. When
two mlops integ builds ran concurrently against the same default bucket,
one build's cleanup wiped the input data the other build was actively
reading, causing spurious "No S3 objects found" / "matched no files on
s3" failures.

Append a uuid suffix to each test's S3 prefix so concurrent builds no
longer share paths and cleanup only removes each run's own data.
…tches in recipe integ tests

Fix three independent failure classes in the sagemaker-train recipe
override / evaluator integ tests, all surfaced under xdist (-n auto).

Evaluator hub resolution: TestBenchMarkEvaluatorRecipeOverrideInteg
constructs BenchMarkEvaluator with the public-hub model
'meta-textgeneration-llama-3-2-1b-instruct' while the session-scoped
use_private_hub fixture pins SAGEMAKER_HUB_NAME to the private 'sdktest'
hub. Unlike the SFTTrainer path, the evaluator's JumpStart model
resolution (_resolve_jumpstart_model) does not fall back to
SageMakerPublicHub, so DescribeHubContent returned ResourceNotFound and
construction failed with a pydantic ValidationError. Override
SAGEMAKER_HUB_NAME to SageMakerPublicHub in both tests, matching the
existing InspectAI tests.

Region isolation: tests that construct SFTTrainer without an explicit
sagemaker_session build a default Session() that reads the region from
the environment. AWS_DEFAULT_REGION was only set as a side effect of the
module-scoped sagemaker_session fixture, so a worker running only such
tests had no region set and Session() raised "Must setup local AWS
configuration with a region supported by SageMaker.". Add a
session-scoped autouse ensure_default_region fixture that pins
AWS_DEFAULT_REGION up front (without clobbering an externally provided
value) so region resolution no longer depends on test order.

Stale error matches: two get_resolved_recipe no-recipe tests matched
"requires a 'recipe' or 'overrides'", but the raised message is now
"requires a 'recipe', 'overrides', or direct hyperparameter
assignments...". Update both regexes to the current wording.
…ructure

The SFTTrainer recipe-override integ tests were written against a
synthetic recipe shape (flat training_config.<hyperparameter>) that
matches the unit-test mocks but not the real Hub recipe. The private
"sdktest" hub does not contain meta-textgeneration-llama-3-2-1b-instruct,
so resolution falls back to SageMakerPublicHub, whose SFT LoRA recipe
nests hyperparameters under training_config.training_args and uses
different field names. As a result overrides were correctly placed (or
intentionally dropped when the key does not exist) while the assertions
read the wrong paths, producing KeyErrors.

Fix the tests to match the real recipe:

- Read hyperparameters under training_config.training_args (learning_rate,
  seed, max_len, etc.) instead of directly under training_config.
- Use real recipe field names: max_epochs (not num_epochs),
  train_batch_size (not batch_size), max_len (not max_length), and the
  non-spec fields micro_train_batch_size / max_norm (not the invented
  max_length / save_top_k).
- Write recipe-file values at their real nested path, since user recipe
  files are not field-name remapped the way the overrides dict is. This
  also makes test_sft_recipe_file_with_invalid_value_raises actually
  reach validation and raise "above maximum".
- Skip test_sft_save_steps_equal_to_max_steps_passes: save_steps/max_steps
  are not part of the llama SFT recipe (it trains by epochs, not steps),
  so the boundary cannot be exercised against this model.

No production code changes: the resolver's field-name remapping and
unknown-key dropping are working as designed and covered by unit tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant